Repository navigation
Conversation
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I found a bounds issue in the new regression fixture: surfaceB is 128 × 128 × 8 = 131,072 bytes, and it starts at address + surfaceA, but bytes (the allocation and registered/imported extent) is only surfaceA + 65,536. That leaves the second half of image B outside the allocated guest block. The readback and final flush still cover the full 131,072-byte image, so the test can access memory beyond the fixture allocation. Could you size/register the block through surfaceA + surfaceB (and keep any intentional overlap explicit), then rerun the focused test?
|
@MarcZX100 thanks for reading the fixture closely. B doesn't start at
The allocation, the guest registration and the host import are all |
|
You are right. I misread the fourth |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Converting this to a draft. With this change I'm seeing a crash during Minecraft Dungeons II level loads (2 of 4 in-game loads on my RTX 4060: a game worker thread reads a garbage pointer, the same function both times), which I haven't seen on |
|
Ready for review again. The crash isn't this change. It's Minecraft Dungeons II's world-load crash from late storage write-backs (#1970, the "Pending storage-image results" entry in
All three crash with the same two signatures: a loading thread or a background worker reads a garbage pointer during the level load. With the hand-over switched off the build does what The PR's numbers on current
On the RTX 4060 the |
|
Gameplay check on the RTX 4060 8 GB with the automatic cache: Minecraft Dungeons II loaded into a level, standing still for 90 s, FPS over the last 60 s. Five alternating rounds of
Storage refreshes are rare in gameplay (about 14 per 10 s, 10 MiB), and the hand-over fired 0 times in every PR row, so the PR doesn't change anything there. The 3.1 to 3.5 rows show up on |
github-actions
Bot
added
perf
|
The on/off byte comparison in Could those paths return a "not tested" result that makes the binary exit 77? At least the watched run's should, since the compared run alone only checks that no hand-over happens. Then a Passed result on CI means the hand-over was exercised. |
23b8ddd to
9371b6b
Compare
|
Done in 1d8072a (now on top of the rebase onto current The rebase also needed a row for |
|
This pull request is labeled |
github-actions
Bot
added
docs
and removed
perf numbers missing
|
Thanks for the exit-77 change, it closes the false pass. One more line is needed so the Linux job accepts the skip.
std::cout << "skipped, the device imports no watched host memory for the clear-key hand-over\n";
return 77;Could |
|
Added in a5dcb10: when the watched run can't run, |
|
Traces of the performance capture in the description (they open in ui.perfetto.dev): |
… clear code A storage image refreshed over memory where other images still had results pending stored those results first, then uploaded. When the image's DCC keys are a clear code its upload is a GPU clear that reads none of that memory, so those stores were dead for it. Unreal's render graph does this whenever it hands transient memory to a new texture it fast-clears. In write-watched memory the results now stay pending. Anything that reads the memory before the image is written stores them as before. Once the image's own results cover their 64 KiB units they are dropped, as its write-back stores every byte of those units; an image written again since keeps its newer results, and a unit stored after the hand-over no longer counts as a CPU store against the image's write. The path is chosen from the keys read before any flush, and only when no flush is needed. APS5_NO_CLEAR_HAND_OVER=1 stores the results first as before.
…iving image's write
a5dcb10 to
278ea58
Compare
github-actions
Bot
added
needs rebase
# Conflicts: # core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
This branch has not been deployed
3 participants
What
StorageTexture::Refreshstores every other image whose results are still pending in the image's memory before it uploads (FlushPending(..., "storage refresh")). When the refreshed image's DCC keys are a clear code, its upload is a GPU clear (upload,IsDccClear+ClearColorFor) that reads none of that memory, so those stores are dead for it. Unreal's render graph hits this constantly: it hands the memory of a transient texture whose results are still pending to a new texture that it fast-clears.On Minecraft Dungeons II (PPSA16064) I counted this at the title screen (RTX 4060, 3072 MiB storage cache, per 10 s): 86,776 storage refreshes, 20,298 of them over memory where other images still had results pending, and 44.3 of those 49.2 GB of pending results (90 %) sat under a refreshed image whose keys were a clear code. None of them reached past the refreshed image. The
storage refreshwrite-backs alone were 53.7 GB per 10 s, about 15 ms of GPU time per frame.perf:
StorageTexture::clearHandOver(called byRefreshbefore its flush) leaves those results pending instead of storing them, when:ProvedKeys()returns a clear code thatClearColorForaccepts.The path is chosen from the keys read before any flush, and only when no flush runs at all, so no store can change the keys between the decision and the upload.
Refreshthen reuses those keys instead of reading them again.What happens to the results afterwards:
markLayersPending), the handed-over results in that unit are dropped, the same way the existing borrow path drops an alias's units: the image's write-back stores every byte of the unit.APS5_NO_CLEAR_HAND_OVER=1stores the results first, as before. WithAPS5_PROFILE_DRAWthe[storage]line counts the hand-overs and the units dropped.test:
agc_storage_clear_hand_over(agc_driver_recorder_tests --clear-hand-over-only). An RGBA8 image A has results over four 64 KiB units; an image B of another format and shape lies over the first two, with DCC keys that are a clear code. Four orders run with the hand-over on and off: A never read again, A read before B writes, A written again after the hand-over, and a CPU store into one of A's units under B before B writes. Each must leave the same texels in B and the same guest bytes as the old order. With the hand-over on, B's refresh must store nothing, A must stay pending, and B's write must drop A's two units under B. The test runs in watched memory (its host import kept watched) and in compared memory, where no hand-over may happen. The test layout is close to the one in perf(agc): hand aliased storage memory over without the guest round trip #4286, which tried a broader hand-over and was closed when it showed no gain in a default configuration.Tested
Windows 10 19045, MinGW GCC, Release. Minecraft Dungeons II (PPSA16064) title screen. FPS is presents per 10 s, averaged over each row's last windows. Rows alternate between
mainandmain+ this commit (mainb39d90b for these runs), with the same relinked executable.RTX 3060 12 GB (i5-12400F, NVIDIA 617.42), default settings (automatic storage budget), 215 s rows (FPS over the last 100 s),
APS5_PROFILE_DRAW=1 APS5_PROFILE_GPU=1:mainRTX 4060 8 GB (i5-14400F, NVIDIA 617.42),
APS5_TEXTURE_CACHE_MIB=3072, 150 s rows (FPS over the last 60 s),APS5_PROFILE_DRAW=1:mainstorage refreshwrite-backs per 10 sOn the RTX 4060 with its automatic budget (1.5-1.8 GB on 8 GB) the frame is bound by storage cache evictions, and this change makes no difference: 3.07 / 3.05 FPS on
mainagainst 3.12 / 2.90 with this PR.On this branch, rebased on
main66e71b6, the 61 tests that runagc_driver_recorder_tests,agc_dcc_key_sharing_testsandagc_driver_sampled_cache_budget_testspass on the RTX 4060. That coversagc_storage_clear_hand_over, everyagc_storage_*test,agc_unit_shadow,agc_pending_key_stores,agc_shadowed_key_stores,agc_dcc_key_sharingandagc_driver_resource_cache_*;agc_import_settle_first_useskips on this machine.tools/check_conventions.py --base origin/mainpasses.Performance capture (as the Performance report form describes): Minecraft Dungeons II (PPSA16064) title screen, RTX 3060 12 GB (i5-12400F), NVIDIA 617.42, Windows 10.0.19045, 3840x2160 output. Only
APS5_TRACE_SECONDS=120:12set, caches warm (the title ran once on the build and was restarted), stderr not redirected.main48d61f5 against this PR's head 9371b6b:Frame and GPU times (ms):
Top phases (ms per frame):
{"commit":"48d61f53b46f","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2158,"ms":12053.995},"presents":99,"frame_ms":{"p50":117.162,"p90":145.047,"p99":158.422,"max":158.422},"gpu_busy_ms_per_present":100.711,"worker_busy_pct":{"queue worker 0x0":99.421,"queue worker 0x20":99.581},"top_phases_ms_per_frame":{"Driver.Packet":242.204,"Driver.Packet.dispatch":86.382,"Driver.Dispatch":85.879,"Driver.Packet.memory_wait":68.067,"Vulkan.Dispatch":40.265,"Vulkan.Dispatch.shader_resources":38.025,"Driver.Packet.device_lock":34.172,"Driver.Packet.prepare_memory":30.126},"trace_dropped":false,"trace":"aps5-trace-48d61f53b46f-9720.json.gz"} {"commit":"9371b6b498e8","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2225,"ms":12046.908},"presents":109,"frame_ms":{"p50":104.904,"p90":144.955,"p99":150.801,"max":151.214},"gpu_busy_ms_per_present":92.270,"worker_busy_pct":{"queue worker 0x0":99.355,"queue worker 0x20":99.627},"top_phases_ms_per_frame":{"Driver.Packet":219.818,"Driver.Packet.dispatch":78.110,"Driver.Dispatch":77.593,"Driver.Packet.memory_wait":60.483,"Vulkan.Dispatch":35.322,"Driver.Packet.device_lock":33.061,"Vulkan.Dispatch.shader_resources":32.803,"Driver.Packet.prepare_memory":25.538},"trace_dropped":false,"trace":"aps5-trace-9371b6b498e8-6060.json.gz"}Traces:
aps5-trace-48d61f53b46f-9720.json.gz(main) andaps5-trace-9371b6b498e8-6060.json.gz(this PR), attached below.trace_digest.py, main
seconds 120:12 from present 2158: 99 presents, frame p50 117.2 / p90 145.0 / p99 158.4 / max 158.4 ms, GPU busy 100.7 ms per present
NVIDIA GeForce RTX 3060, NVIDIA 617.42, Windows 10.0.19045, 3840x2160, commit 48d61f5, dropped: no
{"commit":"48d61f53b46f","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2158,"ms":12053.995},"presents":99,"frame_ms":{"p50":117.162,"p90":145.047,"p99":158.422,"max":158.422},"gpu_busy_ms_per_present":100.711,"worker_busy_pct":{"queue worker 0x0":99.421,"queue worker 0x20":99.581},"top_phases_ms_per_frame":{"Driver.Packet":242.204,"Driver.Packet.dispatch":86.382,"Driver.Dispatch":85.879,"Driver.Packet.memory_wait":68.067,"Vulkan.Dispatch":40.265,"Vulkan.Dispatch.shader_resources":38.025,"Driver.Packet.device_lock":34.172,"Driver.Packet.prepare_memory":30.126},"trace_dropped":false,"trace":"aps5-trace-48d61f53b46f-9720.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.Packet 235.3, batch 100.7, Driver.Packet.dispatch 86.1, Driver.Dispatch 85.2, Driver.Packet.memory_wait 68.0, Vulkan.Dispatch 39.4, Vulkan.Dispatch.shader_resources 37.1, Driver.Packet.device_lock 32.2, Driver.Packet.prepare_memory 27.8, Recorder.Sync 20.7
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
trace_digest.py, this PR
seconds 120:12 from present 2225: 109 presents, frame p50 104.9 / p90 145.0 / p99 150.8 / max 151.2 ms, GPU busy 92.3 ms per present
NVIDIA GeForce RTX 3060, NVIDIA 617.42, Windows 10.0.19045, 3840x2160, commit 9371b6b, dropped: no
{"commit":"9371b6b498e8","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2225,"ms":12046.908},"presents":109,"frame_ms":{"p50":104.904,"p90":144.955,"p99":150.801,"max":151.214},"gpu_busy_ms_per_present":92.27,"worker_busy_pct":{"queue worker 0x0":99.355,"queue worker 0x20":99.627},"top_phases_ms_per_frame":{"Driver.Packet":219.818,"Driver.Packet.dispatch":78.11,"Driver.Dispatch":77.593,"Driver.Packet.memory_wait":60.483,"Vulkan.Dispatch":35.322,"Driver.Packet.device_lock":33.061,"Vulkan.Dispatch.shader_resources":32.803,"Driver.Packet.prepare_memory":25.538},"trace_dropped":false,"trace":"aps5-trace-9371b6b498e8-6060.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.Packet 212.9, batch 91.4, Driver.Packet.dispatch 77.8, Driver.Dispatch 76.9, Driver.Packet.memory_wait 60.4, Vulkan.Dispatch 34.4, Vulkan.Dispatch.shader_resources 31.9, Driver.Packet.device_lock 31.0, Driver.Packet.prepare_memory 23.2, Recorder.WaitSerial 19.9
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)